General whitespace cleanup #7074

respencer · 2024-06-14T14:58:45Z

Changed tabs to spaces
Deleted trailing spaces
Automatically fix code formatting in changed files
Delete some more comments

DawoudIO

Is all of this manual, how can we stop it moving forward

respencer · 2024-06-14T15:38:22Z

Is all of this manual, how can we stop it moving forward

I don't see much chance of this happening in future. Editing software handles things much better now and we have styleci checking on new commits.

DAcodedBEAT · 2024-06-14T21:07:28Z

Is all of this manual, how can we stop it moving forward

I don't see much chance of this happening in future. Editing software handles things much better now and we have styleci checking on new commits.

@respencer if you can, consider adding Generic.WhiteSpace.DisallowTabIndent to the phpcs.xml.dist file

DAcodedBEAT · 2024-06-14T21:11:08Z

src/GroupPropsFormRowOps.php

@@ -32,28 +22,23 @@
 extract(mysqli_fetch_array($rsGroupInfo));

 // Abort if user tries to load with group having no special properties.
-if ($grp_hasSpecialProps == false) {
+if ($grp_hasSpecialProps === false) {


Can you make sure this works? Typically, these values aren't typed correctly when pulled directly from mysqli functions unless the variables are bound (another reason why things should be moved to the ORM)

Not sure right now, so will revert.

src/GroupView.php

src/PeopleDashboard.php

src/skin/Kiosk.css

DAcodedBEAT

We probably don't need to go through each comment to uppercase the first char but since you already did it, I'm not opposed.

The only real concern is the strict equality check change potentially breaking

DAcodedBEAT · 2024-06-14T21:20:37Z

Is all of this manual, how can we stop it moving forward

I don't see much chance of this happening in future. Editing software handles things much better now and we have styleci checking on new commits.

@respencer if you can, consider adding Generic.WhiteSpace.DisallowTabIndent to the phpcs.xml.dist file

Another thing to consider is adding some "best practice" to a .gitattributes file so git can automatically resolve some bad things from entering

DawoudIO

Good PR. Thank you for removing the headers. I was not sure if people would be upset, so I kept them.

I made a few comments nothing big, approving once you address other comment please merge away

DawoudIO · 2024-06-15T22:39:42Z

src/FindFundRaiser.php

-        </table>
-        </td>
+    <form method="get" action="FindFundRaiser.php" name="FindFundRaiser">
+        <input name="sort" type="hidden" value="<?= $sSort ?>" <table cellpadding="3" align="center">


Sorry, why what?

DawoudIO · 2024-06-15T22:40:05Z

src/ChurchCRM/Service/PersonService.php

-        _or()->filterByLastName($searchLikeString, Criteria::LIKE)->
-        _or()->filterByEmail($searchLikeString, Criteria::LIKE)->
-        limit(15)->find();
+        $people = PersonQuery::create()->filterByFirstName($searchLikeString, Criteria::LIKE)->_or()->filterByMiddleName($searchLikeString, Criteria::LIKE)->_or()->filterByLastName($searchLikeString, Criteria::LIKE)->_or()->filterByEmail($searchLikeString, Criteria::LIKE)->limit(15)->find();


i think the original is easier to read

Agreed, automatically reformatted. Thing with moving it back as it was is it will just be automatically reformatted again at some point in the future.

What was the automatic reformatter used? I don't think PHPCS would do this since it didn't every time I ran this prior

PHP Intelephense. Installed phpcs earlier today, will see what that does on this file.

What was the automatic reformatter used? I don't think PHPCS would do this since it didn't every time I ran this prior

phpcs had a lot to complain about, including with how the file was initially. I have yet to figure out how to get phpcbf to care.

And I figured out how to get them all happy. Still have no idea why this wasn't an issue previously.

src/NoteEditor.php

src/SelectDelete.php

src/bin/google-map/GoogleMap.php

- Changed tabs to spaces - Deleted trailing spaces - Automatically fix code formatting in changed files - Delete some more comments

DAcodedBEAT · 2024-06-18T14:03:09Z

src/mysql/upgrade/3.0.0.sql

 DROP TABLE IF EXISTS `menu_links`;
 CREATE TABLE `menu_links` (
-  `linkId` mediumint(8) unsigned NOT NULL AUTO_INCREMENT,
+  `linkId` mediumint(8) UNSIGNED NOT NULL AUTO_INCREMENT,


If you're choosing to capitalize keywords, this should be consistent.

`linkId` MEDIUMINT(8) UNSIGNED NOT NULL AUTO_INCREMENT, `linkName` VARCHAR(50) COLLATE utf8_unicode_ci DEFAULT NULL, `linkUri` TEXT COLLATE utf8_unicode_ci NOT NULL,

I didn't choose anything, the formatter I used did.

Which formatter is this? I don't believe this is documented anywhere and is not in the CI so the formatting was a choice 🤷

Doesn't matter, rolled back.

If you want this formatting to happen and for it to be consistent across devs, we can include it - it would just need to be documented and (if possible) added to the CI

Nope, thank you. It was a bad idea.

Will look at it again at some future date with a different formatter.

src/mysql/upgrade/3.0.0.sql

DAcodedBEAT · 2024-06-18T14:08:06Z

src/mysql/upgrade/2.1.0.sql

+SELECT 0,
+  fam_ID,
+  0,
+  "Original Entry",
+  fam_EnteredBy,
+  fam_DateEntered,
+  "create"
+FROM family_fam;


I'm not the biggest fan of this formatting change - from quick glance, it looks like you're just trying to invoke a select but the select is actually being used as inputs for the insert. This previously was indented to express context and now that context is lost unless the reviewer keeps track of the scope in their head at all times

Will revert.

respencer requested a review from a team as a code owner June 14, 2024 14:58

respencer requested review from DawoudIO, grayeul, DAcodedBEAT and MrClever and removed request for a team June 14, 2024 14:58

respencer force-pushed the whitespace-cleanup branch from 9c93cb4 to 86a0749 Compare June 14, 2024 15:32

DawoudIO approved these changes Jun 14, 2024

View reviewed changes

DAcodedBEAT reviewed Jun 14, 2024

View reviewed changes

src/GroupView.php Show resolved Hide resolved

DAcodedBEAT reviewed Jun 14, 2024

View reviewed changes

src/PeopleDashboard.php Show resolved Hide resolved

DAcodedBEAT reviewed Jun 14, 2024

View reviewed changes

src/skin/Kiosk.css Outdated Show resolved Hide resolved

DAcodedBEAT requested changes Jun 14, 2024

View reviewed changes

DAcodedBEAT added this to the 5.9.0 milestone Jun 14, 2024

DawoudIO approved these changes Jun 15, 2024

View reviewed changes

DawoudIO added the Code Smell label Jun 15, 2024

DawoudIO modified the milestones: 5.9.0, vNext (5.10.0) Jun 15, 2024

respencer force-pushed the whitespace-cleanup branch from 4f6b438 to b1de5f1 Compare June 17, 2024 19:27

General whitespace cleanup

6206687

- Changed tabs to spaces - Deleted trailing spaces - Automatically fix code formatting in changed files - Delete some more comments

respencer force-pushed the whitespace-cleanup branch from b1de5f1 to 6206687 Compare June 18, 2024 12:58

More whitespace fixes

9abdca9

respencer requested a review from DAcodedBEAT June 18, 2024 13:34

DAcodedBEAT reviewed Jun 18, 2024

View reviewed changes

Rollback some changes

5beef70

respencer requested a review from DAcodedBEAT June 18, 2024 15:41

Delete more comments and blank lines

39559b5

respencer force-pushed the whitespace-cleanup branch from 09da784 to 39559b5 Compare June 19, 2024 17:11

respencer merged commit 7118b2e into ChurchCRM:master Jun 19, 2024
3 checks passed

respencer deleted the whitespace-cleanup branch June 19, 2024 17:20

DawoudIO modified the milestones: vNext (5.10.0), 5.9.0 Jul 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

General whitespace cleanup #7074

General whitespace cleanup #7074

respencer commented Jun 14, 2024

DawoudIO left a comment

respencer commented Jun 14, 2024 •

edited by DAcodedBEAT

Loading

DAcodedBEAT commented Jun 14, 2024

DAcodedBEAT Jun 14, 2024

respencer Jun 15, 2024

respencer Jun 18, 2024

DAcodedBEAT left a comment

DAcodedBEAT commented Jun 14, 2024

DawoudIO left a comment

DawoudIO Jun 15, 2024

respencer Jun 17, 2024

DawoudIO Jun 15, 2024

respencer Jun 17, 2024

DAcodedBEAT Jun 18, 2024

respencer Jun 18, 2024

respencer Jun 18, 2024 •

edited

Loading

respencer Jun 18, 2024

DAcodedBEAT Jun 18, 2024

respencer Jun 18, 2024 •

edited

Loading

DAcodedBEAT Jun 18, 2024

respencer Jun 18, 2024

DAcodedBEAT Jun 18, 2024

respencer Jun 19, 2024

DAcodedBEAT Jun 18, 2024

respencer Jun 18, 2024

General whitespace cleanup #7074

General whitespace cleanup #7074

Conversation

respencer commented Jun 14, 2024

DawoudIO left a comment

Choose a reason for hiding this comment

respencer commented Jun 14, 2024 • edited by DAcodedBEAT Loading

DAcodedBEAT commented Jun 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DAcodedBEAT left a comment

Choose a reason for hiding this comment

DAcodedBEAT commented Jun 14, 2024

DawoudIO left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

respencer Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

respencer Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

respencer commented Jun 14, 2024 •

edited by DAcodedBEAT

Loading

respencer Jun 18, 2024 •

edited

Loading

respencer Jun 18, 2024 •

edited

Loading